Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Permit pre-evaluated constants in simd_shuffle #113529

Merged
merged 2 commits into from
Jul 20, 2023
Merged

Permit pre-evaluated constants in simd_shuffle #113529

merged 2 commits into from
Jul 20, 2023

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Jul 10, 2023

fixes #113500

@rustbot
Copy link
Collaborator

rustbot commented Jul 10, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @wesleywiser (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 10, 2023
@rust-log-analyzer

This comment has been minimized.

Sign in to view
cjgillot
cjgillot reviewed Jul 10, 2023
View reviewed changes
compiler/rustc_codegen_ssa/src/mir/constant.rs
// simd_shuffle (passing as argument instead of as a generic param).
rustc_type_ir::ConstKind::Value(valtree) => return Ok(Some(valtree)),
other => span_bug!(constant.span, "{other:#?}"),
},
other => span_bug!(constant.span, "{other:#?}"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also handle ConstantKind::Val?
The fact that const-prop does not replace Unevaluated by Val in this very case is not documented, and may change by accident.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I specifically do not want to handle it. I removed the logic that allows destructuring ConstValue except for pretty printing. I am currently working on eliminating this simd_shuffle special case entirely by making it a real const generic. That should avoid any such issues

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a comment about this.

@Miezhiko
Copy link

tested this patch, it fixes issue for me, at least can build spidermonkey again

@wesleywiser
Copy link
Member

Thanks @oli-obk!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 12, 2023

📌 Commit 5465748 has been approved by wesleywiser

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 12, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 13, 2023
@matthiaskrgr
Rollup merge of rust-lang#113529 - oli-obk:simd_shuffle_evaluated, r=…
83fb198 
…wesleywiser

Permit pre-evaluated constants in simd_shuffle

fixes rust-lang#113500
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 13, 2023
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 13, 2023
@matthiaskrgr
Rollup merge of rust-lang#113529 - oli-obk:simd_shuffle_evaluated, r=…
8701d6b 
…wesleywiser

Permit pre-evaluated constants in simd_shuffle

fixes rust-lang#113500
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 13, 2023
Closed
@matthiaskrgr
Copy link
Member

@bors r-
#113666 (comment)

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 13, 2023
@oli-obk
Copy link
Contributor Author

oli-obk commented Jul 20, 2023

@bors r=wesleywiser

@bors
Copy link
Contributor

bors commented Jul 20, 2023

📌 Commit c7428d5 has been approved by wesleywiser

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 20, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 20, 2023
@matthiaskrgr
Rollup merge of rust-lang#113529 - oli-obk:simd_shuffle_evaluated, r=…
8f7b890 
…wesleywiser

Permit pre-evaluated constants in simd_shuffle

fixes rust-lang#113500
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 20, 2023
Closed
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 20, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 20, 2023
@bors
Auto merge of rust-lang#113890 - matthiaskrgr:rollup-k1w2vii, r=matth…
092e4f4 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#110765 (rustdoc: fix position of `default` in method rendering)
 - rust-lang#113529 (Permit pre-evaluated constants in simd_shuffle)
 - rust-lang#113800 (Avoid another gha group nesting)
 - rust-lang#113827 (Add Foreign, Never, FnDef, Closure and Generator tys to SMIR)
 - rust-lang#113835 (new solver: don't consider blanket impls multiple times)
 - rust-lang#113883 (Remove outdated Firefox-specific CSS for search's crate selector appearance)
 - rust-lang#113884 (Don't translate compiler-internal bug messages)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 8c17e07 into rust-lang:master Jul 20, 2023
@rustbot rustbot added this to the 1.73.0 milestone Jul 20, 2023
@cuviper
Copy link
Member

cuviper commented Aug 16, 2023

Nominating for 1.72-beta to fix #114844, as discussed on zulip.

@rustbot label +beta-nominated

@rustbot rustbot added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Aug 16, 2023
@apiraino
Copy link
Contributor

Beta backport accepted as per compiler team on Zulip

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Aug 17, 2023
@cuviper cuviper mentioned this pull request Aug 18, 2023
Merged
@cuviper cuviper modified the milestones: 1.73.0, 1.72.0 Aug 18, 2023
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Aug 18, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 18, 2023
@bors
Auto merge of rust-lang#114955 - cuviper:beta-next, r=cuviper
7a3a43a 
[beta] backports

* Upgrade std to gimli 0.28.0 rust-lang#114825
* Partially revert rust-lang#107200 rust-lang#114897
* Permit pre-evaluated constants in simd_shuffle rust-lang#113529

r? cuviper
@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Aug 18, 2023
Closed
@oli-obk oli-obk deleted the simd_shuffle_evaluated branch August 21, 2023 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjgillot cjgillot cjgillot left review comments

@wesleywiser wesleywiser wesleywiser approved these changes

Assignees

@wesleywiser wesleywiser

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.72.0
Development

Successfully merging this pull request may close these issues.

ICE when compiling packed_simd_2
10 participants
@oli-obk @rustbot @rust-log-analyzer @Miezhiko @wesleywiser @bors @matthiaskrgr @cuviper @apiraino @cjgillot